Skip to content

feat(Page): add dynamic sticky section support#12409

Open
wise-king-sullyman wants to merge 1 commit intopatternfly:mainfrom
wise-king-sullyman:page-section-add-dynamic-sticky-support
Open

feat(Page): add dynamic sticky section support#12409
wise-king-sullyman wants to merge 1 commit intopatternfly:mainfrom
wise-king-sullyman:page-section-add-dynamic-sticky-support

Conversation

@wise-king-sullyman
Copy link
Copy Markdown
Collaborator

@wise-king-sullyman wise-king-sullyman commented May 4, 2026

What: Closes #12330

Additional issues:

Summary by CodeRabbit

  • New Features

    • Added beta sticky positioning support for Page components with stickyBase and isStickyStuck properties, enabling dynamic sticky behavior based on scroll position.
  • Tests

    • Added comprehensive unit tests for sticky positioning functionality.
  • Documentation

    • Added dynamic sticky section example and usage documentation.
  • Chores

    • Updated PatternFly dependency versions across packages.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 4, 2026

Walkthrough

This PR adds sticky positioning support to PageGroup and PageSection components via new @beta props (stickyBase and isStickyStuck), includes comprehensive test coverage, provides a dynamic scroll-tracking example with a custom hook, and updates the @patternfly/patternfly prerelease version across packages.

Changes

Sticky Positioning Feature

Layer / File(s) Summary
Data Shape
src/components/Page/PageGroup.tsx, src/components/Page/PageSection.tsx
PageGroupProps and PageSectionProps each gain @beta props: stickyBase?: 'top' | 'bottom' and isStickyStuck?: boolean.
Core Implementation
src/components/Page/PageGroup.tsx, src/components/Page/PageSection.tsx
Components destructure new sticky props and conditionally apply CSS modifier classes (stickyTopBase, stickyBottomBase, stickyTopStuck, stickyBottomStuck) based on stickyBase and isStickyStuck.
Example & Hook
src/components/Page/examples/PageDynamicStickySection.tsx
New example component introduces useIsStuckFromScrollParent hook that tracks scroll position via passive scroll event listener and updates isStickyStuck state; breadcrumb section wires this state into the PageSection.
Tests
__tests__/PageGroup.test.tsx, __tests__/PageSection.test.tsx
New test suites verify sticky class behavior: defaults (no sticky classes), base-only application, base+stuck combination, and edge cases where isStickyStuck lacks stickyBase.
Documentation & Dependencies
examples/Page.md, package.json (5 files)
Markdown documentation adds "Dynamic sticky section" subsection and references new example; @patternfly/patternfly version bumped from 6.5.0-prerelease.78 to 6.5.0-prerelease.80 across react-core, react-docs, react-icons, react-styles, and react-tokens.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Possibly related PRs

Suggested labels

Needs design review

Suggested reviewers

  • mcoker
  • kmcfaul
  • thatblindgeye
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(Page): add dynamic sticky section support' accurately summarizes the main change—adding dynamic sticky section support to the Page component via new sticky props and examples.
Linked Issues check ✅ Passed The PR addresses all three objectives from issue #12330: new sticky props added to PageSection/PageGroup [stickyBase, isStickyStuck], new example with generic hook provided (PageDynamicStickySection.tsx), and documentation added to Page.md.
Out of Scope Changes check ✅ Passed All changes are within scope: sticky props for Page components, tests for new functionality, new example component, documentation updates, and patternfly dependency bumps required to support new CSS classes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@patternfly-build
Copy link
Copy Markdown
Collaborator

patternfly-build commented May 4, 2026

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
packages/react-core/src/components/Page/PageGroup.tsx (1)

57-62: ⚡ Quick win

Consider adding a dev warning when isStickyStuck is set without stickyBase

Both components already follow the pattern of warning via useEffect when a prop combination is a silent no-op (e.g., hasOverflowScroll + no aria-label). isStickyStuck=true without stickyBase applies no CSS classes — a console warning would surface this misconfiguration consistently.

💡 Suggested addition (mirroring the existing pattern)
  useEffect(() => {
    if (hasOverflowScroll && !ariaLabel) {
      /* eslint-disable no-console */
      console.warn('PageGroup: An accessible aria-label is required when hasOverflowScroll is set to true.');
    }
+   if (isStickyStuck && !stickyBase) {
+     /* eslint-disable no-console */
+     console.warn('PageGroup: isStickyStuck has no effect without stickyBase also being set.');
+   }
- }, [hasOverflowScroll, ariaLabel]);
+ }, [hasOverflowScroll, ariaLabel, isStickyStuck, stickyBase]);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/react-core/src/components/Page/PageGroup.tsx` around lines 57 - 62,
The component should emit a dev-time console warning when isStickyStuck is true
but stickyBase is not provided because that prop combination is a silent no-op;
inside the existing useEffect block in PageGroup (or a nearby useEffect that
watches prop combos), add a check for isStickyStuck && !stickyBase and call
console.warn with a clear message (e.g., "PageGroup: isStickyStuck is set to
true but stickyBase is not provided — this will have no effect.") so developers
see the misconfiguration; reference the isStickyStuck and stickyBase props and
ensure the effect depends on [isStickyStuck, stickyBase].
packages/react-core/src/components/Page/PageSection.tsx (1)

117-122: ⚡ Quick win

Same isStickyStuck without stickyBase warning applies here.

Same as PageGroup — consider adding a console.warn inside the existing useEffect when isStickyStuck && !stickyBase to keep the DX pattern consistent across both components.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/react-core/src/components/Page/PageSection.tsx` around lines 117 -
122, In PageSection's useEffect add the same developer experience warning as
PageGroup: when isStickyStuck is true and stickyBase is falsy, call console.warn
to inform developers to provide stickyBase; update the existing effect that
currently checks hasOverflowScroll and ariaLabel (function/component:
PageSection, hook: useEffect, props/vars: hasOverflowScroll, ariaLabel,
isStickyStuck, stickyBase) to also perform a conditional console.warn for
isStickyStuck && !stickyBase so the pattern is consistent across components.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/react-core/src/components/Page/PageGroup.tsx`:
- Around line 57-62: The component should emit a dev-time console warning when
isStickyStuck is true but stickyBase is not provided because that prop
combination is a silent no-op; inside the existing useEffect block in PageGroup
(or a nearby useEffect that watches prop combos), add a check for isStickyStuck
&& !stickyBase and call console.warn with a clear message (e.g., "PageGroup:
isStickyStuck is set to true but stickyBase is not provided — this will have no
effect.") so developers see the misconfiguration; reference the isStickyStuck
and stickyBase props and ensure the effect depends on [isStickyStuck,
stickyBase].

In `@packages/react-core/src/components/Page/PageSection.tsx`:
- Around line 117-122: In PageSection's useEffect add the same developer
experience warning as PageGroup: when isStickyStuck is true and stickyBase is
falsy, call console.warn to inform developers to provide stickyBase; update the
existing effect that currently checks hasOverflowScroll and ariaLabel
(function/component: PageSection, hook: useEffect, props/vars:
hasOverflowScroll, ariaLabel, isStickyStuck, stickyBase) to also perform a
conditional console.warn for isStickyStuck && !stickyBase so the pattern is
consistent across components.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a3d6c913-721c-4f8e-a1ef-1f10fbe9301b

📥 Commits

Reviewing files that changed from the base of the PR and between ed21bd6 and 7bad6ee.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (11)
  • packages/react-core/package.json
  • packages/react-core/src/components/Page/PageGroup.tsx
  • packages/react-core/src/components/Page/PageSection.tsx
  • packages/react-core/src/components/Page/__tests__/PageGroup.test.tsx
  • packages/react-core/src/components/Page/__tests__/PageSection.test.tsx
  • packages/react-core/src/components/Page/examples/Page.md
  • packages/react-core/src/components/Page/examples/PageDynamicStickySection.tsx
  • packages/react-docs/package.json
  • packages/react-icons/package.json
  • packages/react-styles/package.json
  • packages/react-tokens/package.json

@wise-king-sullyman wise-king-sullyman requested review from a team, nicolethoen and thatblindgeye and removed request for a team May 4, 2026 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PageSection - add updated sticky props & example

2 participants